Skip to content

add support for riscv64 timer #13324

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

add support for riscv64 timer #13324

wants to merge 2 commits into from
+72 −1

Conversation

heyujiao99
Copy link

Hi, this patch add support for riscv64 timer,

  1. Implemet opal_sys_timer_get_cycles on RISC-V 64 platform using rdtime instruction.
  2. Add a method to read tsc frequency on RISC-V 64 platform.
  3. Correspondingly modified serveral configuration files.
  4. Tested test/util/opal_timer.c on RISC-V hardware,
[root@host util]# ./opal_timer
--> frequency: 50000000 
--> cycle count  
    Slept approximately 50004395 cycles, or 1000087 us 
--> usecs 
    Slept approximately 1000077 us

@heyujiao99
Add RISC64 Timer Implemention.To ensure the timer works properly on t…
5d2bddc 
…he RISC-V platform.

Signed-off-by: heyujiao99 <he.yujiao@sanechips.com.cn>
@jsquyres jsquyres requested review from bosilca and bwbarrett July 4, 2025 11:39
@jsquyres jsquyres added this to the v6.0.0 milestone Jul 4, 2025
bosilca
bosilca reviewed Jul 4, 2025
View reviewed changes
Copy link
Member

@bosilca bosilca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left few comments, but I can't test this and I don't think our CI run on any risc64.

@heyujiao99
use early-clobber in opal_config_asm.m4, remove irrelated code, and u…
54c1e76 
…pdate copyright.

Signed-off-by: heyujiao99 <he.yujiao@sanechips.com.cn>
bosilca
bosilca reviewed Jul 5, 2025
View reviewed changes
Copy link
Member

@bosilca bosilca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is now ready to go. I did not approve it because I don't think we can test it. Do we have a CI or MTT running on risc64 ?

@bwbarrett
Copy link
Member

I agree with George that it would be nice to have testing for RISC-V if we're going to add RISC-V support. We do not currently have anything either as a CI hook or nightly tests. @heyujiao99 is this something that your organization might be able to help out with?

Thanks for addressing George's feedback, but it would also be good to squash your commits before we approve / merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bosilca bosilca bosilca left review comments

@bwbarrett bwbarrett Awaiting requested review from bwbarrett

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
Target: main
Projects
None yet
Milestone
v6.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@heyujiao99 @bwbarrett @bosilca @jsquyres